Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browse filter results on page #4005

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

litvinovg
Copy link
Collaborator

Vitro PR

What does this pull request do?

Adds an option to select search filter to display filtered results on custom page.

What's new?

Please take a look at Vitro PR first.
Updated PageManagement entry form files to align with Vitro PR.

How should this be tested?

A description of what steps someone could take to:

  • Create a new page in PageManagement menu. use type filter as an example
  • Browse page, try selecting facets, alphabetical index, pagination
  • Test the same in non i18n VIVO

Additional Notes:

This change require documentation to be updated.

Interested parties

@hauschke @VIVO-project/vivo-committers

Reviewers' expertise

Candidates for reviewing this PR should have some of the following expertises:

  1. Java
  2. Freemarker

Reviewers' report template

Please update the following template which should be used by reviewers.

General comment

A reviewer should provide here comments and suggestions for requested changes if any.

Testing

A reviewer should briefly describe here how it was tested

Code reviewing

A reviewer should briefly describe here which part was code reviewed

@litvinovg litvinovg linked an issue Sep 16, 2024 that may be closed by this pull request
@chenejac chenejac self-requested a review September 27, 2024 11:27
Copy link
Contributor

@chenejac chenejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General comment

This might be a nice feature to have in VIVO.

Testing

I found a couple of issues:

  • Person filter is not working in my case. Persons are listed on left side, there are numbers in the brackets, but when I click on a person, the list of articles is not displayed (for other filter pages (Organization, Type, Year, Category, it is working)
  • Serbian alphabetical index is not working properly for two letters codes (Dž, Lj, Nj), this should list articles starting with Dž, Lj, or Nj, but it is listing at the moment articles starting with D or Ž (union of lists under D index and Ž index).
  • For some languages alphabetical index is not defined properly (this can be fixed at the end of the review process)

Code reviewing

Didn't review code yet.

@litvinovg
Copy link
Collaborator Author

litvinovg commented Sep 30, 2024

Thank you for testing.

  • Person filter is not working in my case. Persons are listed on left side, there are numbers in the brackets, but when I click on a person, the list of articles is not displayed (for other filter pages (Organization, Type, Year, Category, it is working)

I think Person filter is not configured to include any person related publications in default VIVO configuration


I suppose it should be extended, but I think it is not linked to this PR.

  • Serbian alphabetical index is not working properly for two letters codes (Dž, Lj, Nj), this should list articles starting with Dž, Lj, or Nj, but it is listing at the moment articles starting with D or Ž (union of lists under D index and Ž index).

Thanks. There was an issue with the regular expression I used. It should be fixed now (in Vitro PR). Please try again.

  • For some languages alphabetical index is not defined properly (this can be fixed at the end of the review process)

Sure, I think we will have to ask native speakers to check it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Browse filter results on custom page
2 participants